Skip to content

docs(charter): scheduled fetch as first-class intake + mailbox connection spec (HT-92) - #102

Merged
zaridan merged 5 commits into
mainfrom
docs/ht-92-scheduled-fetch-intake
Jul 21, 2026
Merged

docs(charter): scheduled fetch as first-class intake + mailbox connection spec (HT-92)#102
zaridan merged 5 commits into
mainfrom
docs/ht-92-scheduled-fetch-intake

Conversation

@zaridan

@zaridan zaridan commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Closes HT-92.

Why

Getting email working on a fresh install takes ~26 steps across four consoles. About half exist solely to make Gmail push work, and two of them fail silently — the domain-restricted-sharing org policy blocking the Pub/Sub IAM grant, and a CLI-created subscription missing roles/iam.serviceAccountTokenCreator. There is also no UI for connecting a mailbox at all; the operator runs a raw curl to get a consent URL.

§2's no daemons, no polling loops blocked the obvious fix. This amendment narrows it to what was actually meant.

The charter change

no daemons, no polling loopsno daemons, no long-running processes. Inbound mail may arrive by push webhook or by a bounded, stateless scheduled fetch. Push stays preferred on latency grounds; scheduled fetch becomes a first-class primary transport instead of only a reconciliation backstop.

The original wording conflated two separable things: a daemon (stays resident, holds a connection — what makes FreeScout require a persistent PHP host) and a scheduled fetch (an invocation that ends). Only the first is incompatible with serverless.

The engine already did this. §4 permitted "bounded reconciliation fetches, never a long-running poller," and runGmailWatchMaintenance has always been exactly that — it enqueues the same reconcile job the push path enqueues, and that job reads the mailbox's stored cursor, never the push notification's historyId. Push only makes the same job run sooner. This removes an internal inconsistency rather than granting a new capability.

Verified live before amending

Against the dogfood desk, 2026-07-20:

  • IMAP + app password — authenticated help@resonantiq.app, SELECT INBOX (UIDVALIDITY=1, EXISTS=29), UID FETCH BODY.PEEK[] returned raw RFC822 for 5 messages in 1174 ms, connection closed on exit. A real reply's References chain carried the ht.ht1.… signed reply token intact — the §2 threading anchor holds on this transport.
  • SMTP + the same credentialsmtp.gmail.com:465, AUTH PLAIN, 1833 ms, our own Message-ID preserved verbatim (confirmed by rfc822msgid: lookup at the recipient).

Worth noting: HT-49 exists because Gmail's users.messages.send API rewrites the Message-ID, which is why the reply token moved into References. SMTP submission did not rewrite it. One observation isn't a general rule and the References mechanism works on both — recorded, not acted on.

What this unlocks

Dropping Pub/Sub (six setup steps, both silent-failure traps, and the billing requirement) and IMAP/SMTP connection by app password — no DNS, no MX change, no cloud console, no third-party account. Outbound through the operator's own server means their existing SPF/DKIM already covers deliverability; every provider-webhook alternative considered would have reintroduced it.

What does not change

No process stays resident — no IMAP IDLE, no held connections, no worker loops. Threading authority stays on the outbound Message-ID. Mail semantics stay sacred: any new transport ships with fixtures proving equivalence before becoming a default. The own your data promise is untouched and arguably strengthened — a direct mailbox connection puts fewer third parties in the path than a provider webhook would.

Also in this PR

specs/mail/mailbox-connection.md — transport behaviour (UID cursor + UIDVALIDITY, batch bounding against the 50s maxDuration, BODY.PEEK[] raw-bytes contract), the provider support matrix, credential handling, the connect screen's behavioural contract, and build order.

Microsoft 365 business cannot use app passwords — basic auth was removed and cannot be re-enabled. OAuth is therefore a required second connector, not optional. The HT-40 machinery is reused.

Screen pixels are deliberately not specified — that needs a Claude Design session per the UI fidelity rule.

Reviewer attention

  • The positioning trade in §1 is deliberate and maintainer-approved. We previously sold against FreeScout on "push instead of cron loops watching a mailbox." The honest differentiator survives — FreeScout needs a host running resident scheduler daemons; we run stateless invocations — but the old line was never truthful, since the engine always fetched on a schedule.
  • §8 lists four out-of-scope findings needing their own tickets, including RLS being disabled on all 19 tables (conversations, mailbox_oauth_tokens among them).

Docs only — no code paths touched, no migrations.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Updated product documentation to recognize bounded scheduled fetching as a supported inbound email transport.
    • Clarified serverless operating principles and the distinction between scheduled fetches and long-running processes.
    • Added a mailbox connection specification covering provider setup, IMAP/SMTP checks, test emails, credential handling, and message ingestion.
    • Documented support for app passwords, Microsoft 365 OAuth, generic IMAP, cursor-based fetching, and configurable fetch intervals.
    • Added governance notes and an updated roadmap description.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b58b1b0f-f7a8-48f3-ac5c-0024ae3f76ce

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The charter now recognizes bounded stateless scheduled fetch as a first-class inbound transport. A new mailbox-connection specification defines provider support, connection-screen behavior, IMAP/SMTP transport requirements, credential handling, mailbox addressing, implementation sequencing, and out-of-scope concerns.

Changes

Mailbox connection and scheduled intake

Layer / File(s) Summary
Scheduled-fetch charter contract
CHARTER.md
The charter replaces the “no polling loops” wording with a prohibition on long-running processes, recognizes bounded scheduled fetch, updates the roadmap, and adds amendment HT-92.
Connection and provider behavior
specs/mail/mailbox-connection.md
The specification defines the first-ingestion goal, validated IMAP/SMTP behavior, provider method requirements, and connection-screen checks.
Transport and mailbox operations
specs/mail/mailbox-connection.md
The specification defines cursor-based bounded fetches, UIDVALIDITY tracking, raw RFC822 retrieval, encrypted credentials, configurable intervals, and desk-answer address association.
Implementation plan and scope boundaries
specs/mail/mailbox-connection.md
The build order covers IMAP, SMTP, persistence, verification, Microsoft OAuth, and Gmail push documentation; separate out-of-scope issues are recorded.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the docs-only charter amendment and new mailbox connection spec.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/ht-92-scheduled-fetch-intake

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specs/mail/mailbox-connection.md`:
- Around line 19-21: Update the documentation around runGmailWatchMaintenance
and the scheduled-fetch description to explicitly call this “Gmail scheduled
reconciliation,” not generic scheduled fetch. Clarify that it reconciles Gmail
history using the stored cursor, while retaining the planned IMAP UID-cursor
fetch and adapter work as separate implementation tasks.
- Line 63: Update the “Send test email” specification to define a unique
correlation marker for test messages, use a safe non-customer recipient, and
specify cleanup plus normal-ingestion ignore behavior so test traffic cannot
create production conversations or be processed repeatedly.
- Around line 83-85: Update the app-password lifecycle documentation in the
mailbox connection section to explain that credentials may be invalidated after
a Google account password change or when Advanced Protection blocks app
passwords. Define how invalidation is detected, the reconnect or reauthorization
experience presented to users, and the provider-specific lifecycle differences
while preserving the existing security and write-only requirements.
- Around line 101-108: Update specs/mail/mailbox-connection.md lines 101-108 to
state that scheduled reconciliation removes Pub/Sub as a mandatory setup
dependency while preserving the Gmail push adapter for optional low-latency use.
Update CHARTER.md lines 100-104 to replace “dropping the Pub/Sub dependency”
with wording that identifies scheduled fetch as the default transport and Gmail
push as an optional supported path.
- Around line 74-75: Update the mailbox connection specification to require
invocation-deadline-aware timeouts for IMAP connection, reads, message fetches,
and SMTP operations. Require the worker to stop starting new work before the
deadline and guarantee connection/resource cleanup on success, timeout, and
failure, while retaining the existing batch cap and raw FETCH behavior.
- Line 64: Update the “Errors surface verbatim” requirement in the mailbox
connection documentation to prohibit exposing raw IMAP/SMTP provider errors.
Specify that user-facing responses must use sanitized, escaped messages with
mapped diagnostic codes, while raw provider details are retained only in
redacted server-side diagnostics; preserve the existing mapped common-case
guidance without exposing sensitive account, host, or authentication data.
- Around line 70-75: Clarify the mailbox cron contract so the UID cursor
advances only after each message is successfully and idempotently committed by
the ingestion pipeline, never merely when fetched. Define partial-failure
behavior to retain the last committed UID for retry, and specify that a
UIDVALIDITY change invalidates the stored cursor and rebuilds the boundary by
replaying messages from the new mailbox state.
- Line 29: Revise the “Outbound needs no DNS” statement in the outbound mail
transport documentation to clarify that the operator’s domain must already have
valid SPF/DKIM/DMARC configuration. Limit the claim to avoiding new
Helpthread-owned DNS setup, without implying guaranteed deliverability.
- Line 83: Update the mailbox credential storage requirements to enforce
server-only access, not just encryption. Add explicit deny-by-default RLS
policies or equivalent server-only authorization for mailbox credentials,
including the `mailbox_oauth_tokens` store, and ensure client/API reads cannot
retrieve them while preserving write-only UI behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bb660dbf-bf1d-40c1-a443-49f9a1401334

📥 Commits

Reviewing files that changed from the base of the PR and between aca83cb and b3e796f.

📒 Files selected for processing (2)
  • CHARTER.md
  • specs/mail/mailbox-connection.md

Comment thread specs/mail/mailbox-connection.md Outdated
Comment on lines +19 to +21
**Scheduled fetch already ships.** `runGmailWatchMaintenance` (`src/mail/gmail-watch-maintenance.ts`, the daily cron in `vercel.json`) performs "a bounded reconciliation sweep" that enqueues *the same reconcile job the push path enqueues*. That job (`src/mail/gmail-reconcile.ts`) reads the mailbox's **stored cursor** — explicitly never the push notification's `historyId` — then calls `history.list` followed by `messages.get?format=raw`. It consumes nothing from the push payload.

Consequence: **push only makes the same job run sooner.** Making scheduled fetch the primary intake is a scheduling change, not new fetch code. The lease around `history.list` (`claimReconcileLease`) already prevents concurrent runs from double-fetching.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Do not describe generic scheduled fetch as already shipped.

The cited implementation is Gmail history reconciliation; it does not implement the planned IMAP UID-cursor fetch. Qualify this as “Gmail scheduled reconciliation” and retain the IMAP adapter work in the plan, otherwise implementation may incorrectly be treated as a scheduling-only change.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/mailbox-connection.md` around lines 19 - 21, Update the
documentation around runGmailWatchMaintenance and the scheduled-fetch
description to explicitly call this “Gmail scheduled reconciliation,” not
generic scheduled fetch. Clarify that it reconciles Gmail history using the
stored cursor, while retaining the planned IMAP UID-cursor fetch and adapter
work as separate implementation tasks.

Comment thread specs/mail/mailbox-connection.md Outdated

Worth recording: HT-49 exists because Gmail's `users.messages.send` **API** rewrites the `Message-ID` we set, which is why the reply token had to move into `References` (`src/mail/send.ts`). SMTP submission did not rewrite it. One observation is not proof of a general rule, and the `References` mechanism works on both transports — so this is a point in SMTP's favour, not a reason to change the threading model.

**Outbound needs no DNS.** Because replies go through the operator's own mail server, they are signed by that server's DKIM, from its IPs, on a domain whose SPF already authorises it. Every provider-webhook alternative considered (Postmark, Resend, SES, Cloudflare) would reintroduce SPF/DKIM/DMARC setup. This transport does not.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Qualify the no-DNS deliverability claim.

Using the operator’s SMTP removes Helpthread-owned DNS setup, but it does not guarantee that the operator’s domain has valid SPF/DKIM/DMARC configuration. State the prerequisite explicitly and keep the success criterion limited to avoiding new DNS work.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/mailbox-connection.md` at line 29, Revise the “Outbound needs no
DNS” statement in the outbound mail transport documentation to clarify that the
operator’s domain must already have valid SPF/DKIM/DMARC configuration. Limit
the claim to avoiding new Helpthread-owned DNS setup, without implying
guaranteed deliverability.

- **Provider presets.** Recognise the address's domain and prefill IMAP/SMTP hosts and ports. Unknown domains expand an Advanced section.
- **Method availability is provider-dependent.** For an M365 business domain the app-password option is disabled *with an explanation*, never silently absent.
- **`Check connection`** performs a real IMAP login + `SELECT INBOX` and a real SMTP handshake + `AUTH`, reporting each leg independently. It must never report success from a config-shape check alone.
- **`Send test email`** sends an actual message through the operator's SMTP and confirms it is observed back through the fetch path. This round trip is the single most valuable element on the screen, and it is what today's setup path has no equivalent of — which is why silent misconfiguration currently survives to production.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Isolate test-email traffic from production ingestion.

Define a unique correlation marker, a safe recipient, and cleanup/ignore behavior. Otherwise “Send test email” can create a real customer conversation or be processed repeatedly by the normal intake pipeline.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/mailbox-connection.md` at line 63, Update the “Send test email”
specification to define a unique correlation marker for test messages, use a
safe non-customer recipient, and specify cleanup plus normal-ingestion ignore
behavior so test traffic cannot create production conversations or be processed
repeatedly.

Comment thread specs/mail/mailbox-connection.md Outdated
- **Method availability is provider-dependent.** For an M365 business domain the app-password option is disabled *with an explanation*, never silently absent.
- **`Check connection`** performs a real IMAP login + `SELECT INBOX` and a real SMTP handshake + `AUTH`, reporting each leg independently. It must never report success from a config-shape check alone.
- **`Send test email`** sends an actual message through the operator's SMTP and confirms it is observed back through the fetch path. This round trip is the single most valuable element on the screen, and it is what today's setup path has no equivalent of — which is why silent misconfiguration currently survives to production.
- **Errors surface verbatim** — the provider's actual IMAP/SMTP rejection text, plus a mapped hint for the common cases (2SV not enabled, app passwords blocked by admin policy, wrong port, mailbox not found). Never a bare "connection failed."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not expose provider errors verbatim.

Return a sanitized, escaped user-facing message and a mapped diagnostic code; retain raw provider details only in redacted server-side diagnostics. SMTP/IMAP errors can contain account, host, or authentication details.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/mailbox-connection.md` at line 64, Update the “Errors surface
verbatim” requirement in the mailbox connection documentation to prohibit
exposing raw IMAP/SMTP provider errors. Specify that user-facing responses must
use sanitized, escaped messages with mapped diagnostic codes, while raw provider
details are retained only in redacted server-side diagnostics; preserve the
existing mapped common-case guidance without exposing sensitive account, host,
or authentication data.

Comment on lines +70 to +75
A cron entry alongside the existing four in `vercel.json`. Each invocation: connect, `SELECT INBOX`, fetch messages above the stored UID cursor, hand raw bytes to the pipeline, persist the new cursor, disconnect.

- **No IDLE, no held connections, no resident process.** The connection opens and closes within the invocation. This is the charter constraint, and it must not be optimised away for latency later.
- **Track `UIDVALIDITY` alongside the UID cursor.** If the server changes `UIDVALIDITY`, every stored UID is meaningless and the cursor must be rebuilt. Skipping this silently drops or re-ingests mail.
- **Bound the batch.** `maxDuration` is 50 s (`vercel.json`). Cap messages per invocation and continue on the next tick rather than risk a timeout mid-fetch.
- **Fetch raw.** `FETCH BODY.PEEK[]` — full RFC822, `.PEEK` so `\Seen` is not set. This satisfies `src/providers/inbound-email.ts` natively: raw bytes in, parsed exactly once by `parseInboundEmail`. No IMAP library's convenience parser may touch the message.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Define the cursor commit point.

Advance the UID cursor only through successfully committed, idempotently ingested messages, not merely through fetched messages. A partial pipeline failure followed by cursor advancement can permanently skip mail; UIDVALIDITY reset behavior should also specify the rebuild/replay boundary.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/mailbox-connection.md` around lines 70 - 75, Clarify the mailbox
cron contract so the UID cursor advances only after each message is successfully
and idempotently committed by the ingestion pipeline, never merely when fetched.
Define partial-failure behavior to retain the last committed UID for retry, and
specify that a UIDVALIDITY change invalidates the stored cursor and rebuilds the
boundary by replaying messages from the new mailbox state.

Comment thread specs/mail/mailbox-connection.md Outdated
Comment on lines +74 to +75
- **Bound the batch.** `maxDuration` is 50 s (`vercel.json`). Cap messages per invocation and continue on the next tick rather than risk a timeout mid-fetch.
- **Fetch raw.** `FETCH BODY.PEEK[]` — full RFC822, `.PEEK` so `\Seen` is not set. This satisfies `src/providers/inbound-email.ts` natively: raw bytes in, parsed exactly once by `parseInboundEmail`. No IMAP library's convenience parser may touch the message.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound every network operation by the invocation deadline.

A message-count cap does not prevent one IMAP connection, fetch, or SMTP operation from consuming the 50-second limit. Specify connect/read/fetch deadlines, stop starting work before the deadline, and guarantee cleanup.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/mailbox-connection.md` around lines 74 - 75, Update the mailbox
connection specification to require invocation-deadline-aware timeouts for IMAP
connection, reads, message fetches, and SMTP operations. Require the worker to
stop starting new work before the deadline and guarantee connection/resource
cleanup on success, timeout, and failure, while retaining the existing batch cap
and raw FETCH behavior.


### Credentials

App passwords are long-lived secrets granting full mailbox access. They must be encrypted at rest via the existing `HELPTHREAD_TOKEN_ENC_KEY` path (`src/store/token-crypto.ts`, AES-256-GCM) already used for OAuth tokens, never logged, never returned by any API read, and write-only in the UI — show a "configured" state, never the value.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Make credential access server-only, not merely encrypted.

AES-GCM protects stored ciphertext but does not prevent unauthorized reads. This same document records RLS disabled on mailbox_oauth_tokens; require server-only access or explicit deny-by-default policies before storing mailbox credentials.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/mailbox-connection.md` at line 83, Update the mailbox credential
storage requirements to enforce server-only access, not just encryption. Add
explicit deny-by-default RLS policies or equivalent server-only authorization
for mailbox credentials, including the `mailbox_oauth_tokens` store, and ensure
client/API reads cannot retrieve them while preserving write-only UI behavior.

Comment on lines +83 to +85
App passwords are long-lived secrets granting full mailbox access. They must be encrypted at rest via the existing `HELPTHREAD_TOKEN_ENC_KEY` path (`src/store/token-crypto.ts`, AES-256-GCM) already used for OAuth tokens, never logged, never returned by any API read, and write-only in the UI — show a "configured" state, never the value.

Note the asymmetry worth telling operators about: an app password cannot be scoped and does not expire; an OAuth token is scoped to `gmail.readonly` + `gmail.send` and is revocable from the account.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Document app-password revocation and reauthorization.

“Does not expire” is too absolute: Google app passwords can be revoked after a password change and are blocked under Advanced Protection. Define invalidation detection, reconnect UX, and provider-specific lifecycle semantics. (support.google.com)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/mailbox-connection.md` around lines 83 - 85, Update the
app-password lifecycle documentation in the mailbox connection section to
explain that credentials may be invalidated after a Google account password
change or when Advanced Protection blocks app passwords. Define how invalidation
is detected, the reconnect or reauthorization experience presented to users, and
the provider-specific lifecycle differences while preserving the existing
security and write-only requirements.

Source: MCP tools

Comment thread specs/mail/mailbox-connection.md Outdated
Comment on lines +101 to +108
1. **Drop the Pub/Sub dependency** — reschedule the existing reconcile sweep. Least new code, largest reduction in setup steps, removes both silent-failure traps and the billing requirement (Pub/Sub is what forced it).
2. **IMAP provider adapter** behind the existing `InboundEmailProvider` seam, with fixtures proving equivalence against the Gmail path.
3. **Migrations** — UID cursor + `UIDVALIDITY`; encrypted mailbox credential.
4. **SMTP sender** behind the existing `EmailSender` seam.
5. **The connection screen** — design session first, then implementation.
6. **`Check connection` / `Send test email`**, including the round-trip verification.
7. **Microsoft OAuth connector**, reusing the HT-40 machinery.
8. Demote Gmail push in the docs to the advanced low-latency option; keep it fully supported.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Clarify that Pub/Sub becomes optional, not unsupported.

The charter still says Gmail push is preferred/supported, while both documents say “drop the Pub/Sub dependency.” Specify that scheduled fetch removes Pub/Sub as a mandatory setup requirement but preserves the Gmail push adapter and path for operators who want low latency.

  • specs/mail/mailbox-connection.md#L101-L108: revise the build order to remove mandatory Pub/Sub setup, not Gmail push support.
  • CHARTER.md#L100-L104: replace “dropping the Pub/Sub dependency” with wording that distinguishes optional push support from the default scheduled-fetch transport.
📍 Affects 2 files
  • specs/mail/mailbox-connection.md#L101-L108 (this comment)
  • CHARTER.md#L100-L104
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/mailbox-connection.md` around lines 101 - 108, Update
specs/mail/mailbox-connection.md lines 101-108 to state that scheduled
reconciliation removes Pub/Sub as a mandatory setup dependency while preserving
the Gmail push adapter for optional low-latency use. Update CHARTER.md lines
100-104 to replace “dropping the Pub/Sub dependency” with wording that
identifies scheduled fetch as the default transport and Gmail push as an
optional supported path.

zaridan and others added 5 commits July 21, 2026 09:21
…tion spec (HT-92)

§2's "no daemons, no polling loops" is restated as "no daemons, no
long-running processes." Inbound mail may now arrive either by push
webhook or by a bounded, stateless scheduled fetch — connect, read from
a stored cursor, exit. Push stays preferred where a provider offers it;
scheduled fetch becomes a first-class primary transport rather than only
a reconciliation backstop.

The original wording conflated a daemon (stays resident, holds a
connection — what makes FreeScout need a persistent PHP host) with a
scheduled fetch (an invocation that ends). Only the first is
incompatible with serverless. §4 already permitted "bounded
reconciliation fetches, never a long-running poller," and the daily
watch-maintenance cron has always been exactly that: it enqueues the
same reconcile job the push path enqueues, and that job reads the
mailbox's stored cursor, never the push notification's historyId. The
engine already did on a schedule what §2 appeared to forbid.

Verified live against the dogfood desk before amending:
- IMAP + app password authenticated help@resonantiq.app, returned raw
  RFC822 via BODY.PEEK[], and a real reply's References chain carried
  the ht.ht1.… signed token intact — the §2 threading anchor holds.
- SMTP + the same credential sent with our own Message-ID preserved
  verbatim, confirmed by rfc822msgid: lookup at the recipient.

Unlocks dropping the Pub/Sub dependency (six setup steps, including the
two that fail silently) and IMAP/SMTP connection by app password — no
DNS, no MX change, no cloud console, no third-party account.

Consequential wording updated at the opening summary, the FreeScout
comparison, the serverless positioning line, and §4's Phase 1.

Adds specs/mail/mailbox-connection.md: transport behaviour, provider
support matrix (Microsoft 365 requires OAuth — app passwords are
unreachable there), credential handling, the connect screen's
behavioural contract, and build order. Screen pixels are deferred to a
Claude Design session per the UI fidelity rule.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ent (HT-92)

Three independent reviews (two Opus-tier, one Codex) attacked this PR. The
amendment's central claim survived: the daily cron genuinely already
enqueued the same reconcile job on the same topic, and the consumer reads
the stored cursor, so this removes an internal inconsistency rather than
introducing a capability. The findings below are the ones that landed.

**The repo contradicted the charter.** The amendment updated CHARTER.md and
nothing else, leaving "no daemons, no polling loops" quoted as binding
authority in four other places — including README.md, the public front door,
which directly contradicted the charter it links to. Also corrected in
src/providers/inbound-email.ts, src/providers/queue.ts, and
specs/modules/catalog.md, whose "no IMAP polling" rationale for dropping a
FreeScout feature is now void and flagged for re-deciding on its merits.

**The spec is demoted to DRAFT.** Review found three gaps in
specs/mail/mailbox-connection.md, all sitting on the mail-semantics
invariant, all of which block its own build order:

1. Self-echo suppression has no mechanism on SMTP+IMAP. preSuppressOwnSend
   pre-seeds the ledger with the send's providerMessageId — "the SAME id the
   transport will later report." Gmail's API supplies that shared id; SMTP
   submission returns no id a later IMAP FETCH will report. Without an
   answer, every agent reply is re-ingested as an inbound customer message.
   HT-49/HT-50 were this class of bug and were found live.
2. providerMessageId is undefined for IMAP. The ledger's idempotency key
   requires a stable per-transport id; the spec itself notes a UIDVALIDITY
   change makes every stored UID meaningless, so a UID-keyed ledger
   re-ingests the whole mailbox on reset.
3. InboundEmailProvider is webhook-shaped — both methods take an HTTP
   Request, which a cron-driven fetch does not have. The claims "least new
   code" and "behind the existing seam" rested on this and were wrong.

These are recorded in the spec rather than papered over. §2's spike results
and §3's provider matrix are independently verified and stand.

**Smaller corrections.** "It consumes nothing from the push payload" was
literally false — it consumes mailboxId; the payload's historyId is logged
and never acted on. Noted too that the field carries semantically opposite
values depending on trigger, which is a latent trap. The amendment was also
inserted mid-appendix, breaking a strictly chronological run; moved to the
end after HT-75.

Verified: tsc exit 0, biome exit 0, 1512 tests pass across 75 files.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(HT-92)

Six of nine actionable findings taken; three were wording preferences,
answered on the PR rather than applied. All land on the spec — the charter
amendment drew none.

Two were claims that were wrong, not merely thin:

- "Outbound needs no DNS" overclaimed. Sending through the operator's own
  server inherits whatever SPF/DKIM/DMARC that domain already has; it does
  not guarantee deliverability. A domain with no SPF or a broken selector
  delivers just as badly through Helpthread as through its owner's normal
  client. The true claim is that we add no NEW DNS burden, unlike every
  provider-webhook alternative.
- "Errors surface verbatim" was a leak. Raw IMAP/SMTP rejection text
  routinely carries hostnames, account addresses and auth detail; echoing it
  to a browser is not acceptable. Errors are now mapped to sanitized
  messages with diagnostic codes, raw text to redacted server logs only.

Four were real gaps:

- `Send test email` could manufacture a customer conversation. It now must
  address the connected mailbox itself (never an operator-typed recipient,
  so the setup screen cannot become a way to mail third parties), carry a
  correlation nonce the pipeline drops on, and be rate-limited.
- No timeouts were specified. Every IMAP/SMTP operation now carries a
  timeout derived from the remaining invocation budget, the worker stops
  starting work it cannot finish, and connections close on success, timeout
  and failure alike. Without this one hung FETCH burns the whole invocation,
  every minute, indefinitely.
- The UID cursor advances on COMMIT, not on fetch, and holds at the last
  committed UID on partial failure. Re-fetching a committed message is
  harmless because ingest is idempotent; advancing past an uncommitted one
  loses mail silently, which §2 does not permit.
- App-password lifecycle was undocumented. They die quietly — no revocation
  signal, no refresh failure — when the account password changes, an admin
  disables them, or the account enrols in Advanced Protection. Auth failure
  now maps to the same `needs_reconnect` state a dead OAuth grant produces,
  and the reconnect path differs per provider (credential re-entry, not a
  consent redirect).

Also noted: encrypting the credential at rest is not sufficient without
deny-by-default server-only authorization — the same gap already open on
mailbox_oauth_tokens, tracked in §8.

The spec remains DRAFT; these are quality fixes to a document still blocked
on its three mail-semantics questions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
I triaged these two as wording preferences and skipped them. Both are
labelled Major and both are real; one is an overclaim repeated throughout
this PR and in the surrounding discussion.

**"Scheduled fetch already ships" was misleading.** What ships is GMAIL
history reconciliation — a cursor-based history.list walk against the Gmail
API. That proves cursor-driven, cron-triggered intake is architecturally
sound and running in production, which is what the amendment rests on. It
provides nothing toward the IMAP UID-cursor fetch this spec describes: that
adapter, its cursor semantics, its UIDVALIDITY handling and its idempotency
key are all new and substantial work. The old phrasing invited reading the
whole thing as a scheduling-only change. Now stated as "the pattern is
proven," never "the transport is built."

**"Drop the Pub/Sub dependency" read as removing support.** Both documents
said drop while the charter says push stays preferred — a direct
contradiction. Pub/Sub becomes optional, not unsupported: the push adapter,
the webhook and the watch() renewal all remain fully supported for
operators who want sub-minute latency. What changes is that they stop being
mandatory setup. Corrected in the spec's build order and in the amendment's
"what it unlocks" paragraph, and the build-order item now also notes it is
Gmail-only and does nothing for IMAP.

All nine CodeRabbit findings on this PR are now addressed. Every one landed
on the spec; the amendment itself drew none.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…T-92/HT-100)

The amendment as drafted elevated scheduled fetch to a 'first-class primary
transport, not merely a reconciliation backstop.' That framing was the drafting
assistant's, not TJ's — it was explicitly named as his call and then made for
him ('That's a business call, not an engineering one, and it's yours').

On review TJ pointed out the framing is wrong at the root: there is no
system-wide default, because the operator picks the transport at setup. Verified
against specs/deploy/gmail-inbound-runbook.md Part A — Gmail push needs a GCP
project, an Internal OAuth app, a Pub/Sub topic, a Gmail publisher grant, a
service account, and an OIDC push subscription, plus two documented org-policy
landmines hit during live provisioning. Scheduled fetch needs an app password or
OAuth. The easier path wins on adoption regardless of what any document calls
primary.

So neither is designated primary. Both are fully supported intake paths and the
charter now states the tradeoff plainly — latency against setup cost — and
leaves the choice with the operator.

TJ's approved §1 positioning line is unchanged and never claimed a primary:
'push-based delivery where providers offer it, bounded scheduled fetches where
they don't — and no resident process either way.'

The §7 appendix amendment now records what he actually ratified, with his own
words, and notes that the 'first-class primary transport' framing was removed as
assistant-authored. Two stale 'primary' claims in the mailbox-connection draft
reconciled to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@zaridan
zaridan force-pushed the docs/ht-92-scheduled-fetch-intake branch from a5bd06c to 7e76182 Compare July 21, 2026 16:30
@zaridan
zaridan merged commit e663e4d into main Jul 21, 2026
5 checks passed
@zaridan
zaridan deleted the docs/ht-92-scheduled-fetch-intake branch July 21, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant